Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --slug=<site> as an available parameter to wp site commands #416

Merged
merged 17 commits into from
Sep 18, 2023

Conversation

tubiz
Copy link
Contributor

@tubiz tubiz commented Aug 24, 2023

Fixes #257

Slug has been added as an available parameter for the wp site commands listed below

  • archive wp site archive --slug={siteslug}
  • activate wp site activate --slug={siteslug}
  • deactivate wp site deactivate --slug={siteslug}
  • mature wp site mature --slug={siteslug}
  • private wp site private --slug={siteslug}
  • public wp site public --slug={siteslug}
  • spam wp site spam --slug={siteslug}
  • unarchive wp site unarchive --slug={siteslug}
  • unmature wp site unmature --slug={siteslug}
  • unspam wp site unspam --slug={siteslug}

Related wp-cli/wp-cli#5832

@tubiz tubiz requested a review from a team as a code owner August 24, 2023 18:36
@schlessera
Copy link
Member

Great work so far, @tubiz !

Are you still up for adding feature tests for this?

@tubiz
Copy link
Contributor Author

tubiz commented Aug 24, 2023

Great work so far, @tubiz !

Are you still up for adding feature tests for this?

Yes. I will get started with adding the feature tests.

Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include some tests covering this change? Also, it'd be great to abstract the site fetcher to a dedicated method.

@danielbachhuber danielbachhuber added this to the 2.5.4 milestone Aug 27, 2023
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting better! I think there's just one last bit to address.

$this->update_site_status( [ $blog->blog_id ], 'archived', 1 );
} else {
$this->update_site_status( $args, 'archived', 1 );
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For all of these, we also need to provide some error mechanism when neither <id> nor --slug= are provided. Check out wp plugin activate and its sibling commands for an example of how this can be implemented:

https://github.com/wp-cli/extension-command/blob/6a68d247969b8968b007a4d664e0de14bf69e775/src/Plugin_Command.php#L317-L320
https://github.com/wp-cli/extension-command/blob/6a68d247969b8968b007a4d664e0de14bf69e775/src/WP_CLI/ParsePluginNameInput.php#L9-L47

We'll want to add tests for this aspect too.

Your get_site_by_slug() method could be updated to $this->get_sites( $args, $assoc_args ) and return a $blogs variable that's passed directly into $this->update_site_status()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update get_site_by_slug() method to get_sites_ids()

The check to ensure that the site IDs or slug are provided is now done in the check_site_ids_and_slug() method.

@danielbachhuber danielbachhuber changed the title Add slug as an available parameter to additional 'wp site' commands Add --slug=<site> as an available parameter to wp site commands Sep 18, 2023
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this, @tubiz !

I made one more tweak with 24d2cbe

@danielbachhuber danielbachhuber merged commit 216a6a1 into wp-cli:main Sep 18, 2023
32 checks passed
@tubiz tubiz deleted the slug-parameter-support branch September 20, 2023 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for site archive and site deactivate by slug
3 participants